Skip to content

ASCReader: Improve datetime parsing and support double-defined AM/PM cases#2009

Merged
zariiii9003 merged 5 commits intohardbyte:mainfrom
Gedlex:enhanced-asc-datetime-parsing
Dec 2, 2025
Merged

ASCReader: Improve datetime parsing and support double-defined AM/PM cases#2009
zariiii9003 merged 5 commits intohardbyte:mainfrom
Gedlex:enhanced-asc-datetime-parsing

Conversation

@Gedlex
Copy link
Contributor

@Gedlex Gedlex commented Nov 27, 2025 •
edited
Loading

Summary of Changes

The datetime handling has been improved to robustly handle double-defined datetime strings where both a 24h timestamp and an AM/PM suffix are present (e.g. "30 15:06:13.191 pm 2017") which previously caused parsing to fail.

Key improvements:

  • Added support for “double-defined” datetime strings
  • Cleaned up month replacement logic using a safer KeyError-handling path
  • Added clearer error messages for unknown months and incompatible formats

A small unit test was used during development:

from can.io.asc import ASCReader

# Define test strings
test_strings = ["May 27 04:09:35.000 pm 2014",
                "Mai 27 04:09:35.000 pm 2014",
                "Apr 28 10:44:52.480 2022",
                "Apr 28 10:44:52.480 am 2022",
                "Sep 30 15:06:13.191 2017",
                "Sep 30 15:06:13.191 pm 2017", # double defined case
                "Sep 30 15:06:13.191 am 2017", # technically invalid case
                ]

# Test function
for string in test_strings:
    res = ASCReader._datetime_to_timestamp(string)
    print(f"Timestamp for {string} is {res}")

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • Other (please describe):

Checklist

  • I have followed the contribution guide.
  • I have added or updated tests as appropriate.
  • I have added or updated documentation as appropriate.
  • I have added a news fragment for towncrier.
  • All checks and tests pass (tox).

@zariiii9003
Copy link
Collaborator

zariiii9003 commented Nov 30, 2025

Please use black to fix the code formatting and add a news fragment.

Also, here's a test you could add to test/logformats_test.py::TestAscFileFormat

    @parameterized.expand(
        [
            ("May 27 04:09:35.000 pm 2014", 1401199775.0),
            ("Mai 27 04:09:35.000 pm 2014", 1401199775.0),
            ("Apr 28 10:44:52.480 2022", 1651135492.48),
            ("Sep 30 15:06:13.191 2017", 1506776773.191),
            ("Sep 30 15:06:13.191 pm 2017", 1506776773.191),
            ("Sep 30 15:06:13.191 am 2017", 1506776773.191),
        ]
    )
    def test_datetime_to_timestamp(self, datetime_string: str, expected_timestamp: float):
        timestamp = can.ASCReader._datetime_to_timestamp(datetime_string)
        self.assertAlmostEqual(timestamp, expected_timestamp)

@Gedlex
Copy link
Contributor Author

Gedlex commented Dec 1, 2025

Thank you for the detailed feedback. I updated the formatting as requested and added your test case to test/logformats_test.py::TestAscFileFormat.

zariiii9003
zariiii9003 approved these changes Dec 2, 2025
View reviewed changes
Copy link
Collaborator

@zariiii9003 zariiii9003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@zariiii9003 zariiii9003 merged commit 2630fb3 into hardbyte:main Dec 2, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zariiii9003 zariiii9003 zariiii9003 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Gedlex @zariiii9003